Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve iPython interoperability #7268

Closed
wants to merge 7 commits into from

Conversation

smason
Copy link
Contributor

@smason smason commented Jul 7, 2023

Fixes #7259

Changes proposed in this pull request:

  • support _repr_mimebundle_ by returning either PNG or JPEG
  • convert images to a supported mode where possible
  • shrink large images by an integer factor
  • output warning messages when changes were made for display purposes
  • refactor _repr_png|jpeg_ to do the same pre-processing
  • because large are resized it made sense to remove the previously requested compression_level=1

I've been finding the current behavior to be unusable in a Jupyter notebook. The number of dumped backtraces are very painful to work with, so I've tidied up my method in #7259 and incorporated it into the codebase. Until I get a response to my question from the IPython project I think this code does reasonable things.

@radarhere
Copy link
Member

This conversation should be re-evaluated if/when someone from the IPython team responds, but for the moment, I'm not really in favour of the various automatic changes you're making here.

In particular, you've added scaling for I;16 images, which is a change regarding #3159 that would occur here, but nowhere else.

For an analogous situation, in #7241, you were concerned about the speed of saving images for IPython. I was in favour of #7242 as a solution because it mirrored the behaviour of ImageShow. I think any arguments you make here for IPython would similarly apply to ImageShow.

Different users might have different preferences for how much they want images to be scaled down by. I know you've added a constant to try and make this configurable - my feeling is just that it is better to let the user specify what they would like, rather than guessing. If I found images displaying at a small size, I would first believe that the image itself was small, rather than expecting a library to be transforming it automatically.

Those are just my thoughts though. You're welcome to leave this open, and if someone else approves of this plan, I raise no objections.

@smason
Copy link
Contributor Author

smason commented Jul 7, 2023

Mostly did this so I could have a current version that installed locally that behaved better with Jupyter. Happy to leave it open until some decision can be reached.

Re, the I;16 handling, I hadn't tested with loading a 16bit PNG. I've used JPEG2000 more when dealing with higher bit-depth images, and assumed other formats were treated similarly. For example:

Image.fromarray(np.random.randint(65536, size=(100, 100), dtype='uint16')).save('out.jp2')
print(im.open('out.jp2').mode)

gives me I;16 which seems useful. This is similar when using TIFF files.

I've made it print warnings so user has some indication of when it's making non-trivial changes. But I agree that I've baked lot of my preferences into policy rather than providing the mechanisms and letting the user define their preferred policy.

I've been reading more of their API, and have seen that you can use BaseFormatter.for_type to register custom code. Which might be a better way of handling things, something like:

from IPython.display import display
def show_image(obj, *args, **kwargs):
    display({
        'text/plain': f"custom={str(obj)}",
        # 'image/png': b'...',
        # 'image/jpeg': b'...',
    }, raw=True)

get_ipython().display_formatter.ipython_display_formatter.for_type(Image.Image, show_image)

Image.new('L', (10, 10))

and have some method available that has some common policy, but could be swapped out with another formatter as required. The issue is that this seems to be even more entangled in the guts of IPython.

@aclark4life
Copy link
Member

@n3011 Any opinion here?

Expose Image.use_display_hook_features that can be used to control how
images are seen in IPython / Jupyter.  For example:

  Image.use_display_hook_features(F_mode='unit')

special cases F mode images, by assuming values are in [0., 1.].
Alternatively, the user could do:

  Image.use_display_hook_features('auto')

and it will function similarly to my previous patch.
@smason
Copy link
Contributor Author

smason commented Jul 13, 2023

Following all the activity on other related issues, I've had a go at reworking this change. By default it will now only return either a PNG or JPEG image, as determined by whichever encodes to less bytes. It won't do any other conversion.

I've added a Image.use_display_hook_features method that can be called to optionally enable various transforms to make the image more compatible with IPython/Jupyter. For example:

Image.use_display_hook_features(F_mode='unit', mode_map='auto')

would allow the following images to be displayed automatically:

Image.new('HSV', (400, 400))
Image.fromarray(np.random.random((400, 400)))

Or you can mostly "full auto" mode with:

Image.use_display_hook_features('auto', F_mode='unit')

which also resizes large images and other things.

Interested to hear opinions!

@homm
Copy link
Member

homm commented Jul 14, 2023

It's a quite big change and complicated API, especially when I still don't see any explanation why _repr_jpeg_ is useful for someone.

@radarhere
Copy link
Member

A comment from the author of this PR - #7259 (comment)

I'm not sure if there's enough interest to maintain the relevant state in Pillow directly.

Specifically, I'm dealing with raw camera data (i.e. lots of pixels at 12 or 14bpc) and Pillow doesn't seem to track the numeric range from the source data in a way that would help me. I'm also interested in just previewing images while I'm exploring the data in Jupyter (hence my code downsampling images) and returning both PNG and JPEG versions of the image seems wasteful.

When I've got a workflow/code that's mostly correct I'll turn it into a proper Python module that's tested to process things correctly, but, e.g., while working out whether I'm correctly debayering an image correctly I just want a quick preview that things are on track. In my experience this is a good way to use Jupyter notebooks; exploratory work goes into notebooks where it's very quick to iterate on changes (no need to rerun Python and/or reload large datasets), then the code is tidied up for production into a module (or a package for large chunks of code).

You can close as Wont Fix if you think that's relevant, I'm not sure how many other people use Pillow in the way I am.

As I've indicated earlier, I think this is somewhat too... automatic. This PR is trying to solve #3159, #2663, and also scaling images automatically, but only for IPython, while against an IPython decision. It's trying to more IPython use easier, but the boundaries for this change are irregularly defined enough that I think this should be code on the user's end, rather than something built into Pillow.

@radarhere radarhere closed this Aug 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Addition of _repr_jpeg_ causes both JPEG and PNG format images to be attached to Jupyter cells
4 participants